Add pagination to user following/follower lists #9323
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
This is a great start @pidgezero-one, great work so far. You might consider running it using |
pidgezero-one
commented
May 25, 2024
| i = web.input(page=1) | ||
| page_size = 25 | ||
| page = 1 if not i.page.isdigit() else max(1, int(i.page)) | ||
| offset = (page - 1) * page_size |
There was a problem hiding this comment.
Since the page is also used to calculate the offset, I validate it here and pass it into the template instead of defining in the template. The number of values being passed to the template is quite large, and this might not be totally necessary, but doing it this way lets me force invalid page values in the query param to default to page 1 (i.e. ?page=0, ?page=-1, ?page=aaaa all default to the first page) before using it to calculate the offset.
scottbarnes
added
the
Needs: Review
This was referenced May 29, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #9283
Adds pagination support to
people/:username/followersandpeople/:username/followingTechnical
results_per_pageas a mandatory variable in the template instead of optional, so that the page size given to the macro will always match the page size used for calculating the offset in the DB queryfollowsto the template, but I don't think returning the whole DB contents would be solving an issue of performance as well as returning a constant 25 or fewer rows does.get_following. I looked in other parts of the codebase to verify that None and 0 are appropriate defaults.Testing
I inserted dummy data into the
followstable to verify that both lists loaded correctly at 0, 1-25, and 26+ entries, and verified that the pagination UI loads the correct section of the list depending on the page (http://localhost:8080/people/openlibrary/followers, http://localhost:8080/people/openlibrary/followers?page=1, http://localhost:8080/people/openlibrary/followers?page=2).Screenshot
These are zoomed out.
Following list, 25 entries in total:
Following list, 26 entries:
Following list, empty:
Followers list, 25 entries in total:
Followers list, 26 entries:
Followers list, empty:
Stakeholders
@mekarpeles
Attribution Disclaimer: By proposing this pull request, I affirm to have made a best-effort and exercised my discretion to make sure relevant sections of this code which substantially leverage code suggestions, code generation, or code snippets from sources (e.g. Stack Overflow, GitHub) have been annotated with basic attribution so reviewers & contributors may have confidence and access to the correct context to evaluate and use this code.